Append AZURE_HTTP_USER_AGENT environment variable to UserAgent string. #483
Conversation
There was a problem hiding this comment.
Hey @jeffreyCline
Thanks for this PR :)
I've taken a look and it mostly LGTM (I've left a couple of minor comments around our conventions going forward - but they're not blockers by any means). Out of interest, where's this environment variable going to be set?
Thanks!
azurerm/config.go
Outdated
| @@ -189,7 +191,15 @@ func withRequestLogging() autorest.SendDecorator { | |||
|
|
|||
| func setUserAgent(client *autorest.Client) { | |||
| version := terraform.VersionString() | |||
| azureAgent := os.Getenv("AZURE_HTTP_USER_AGENT") | |||
There was a problem hiding this comment.
out of interest - what's the use-case for this / where's this going to be set?
There was a problem hiding this comment.
The azure compute cloud shell sets this in their dockerfile (i.e. ENV AZURE_HTTP_USER_AGENT cloud-shell/1.0). This way we can collect some BI data on the provider/environment usage.
azurerm/config.go
Outdated
|
|
||
| if azureAgent != "" { | ||
| s := []string{client.UserAgent, azureAgent} | ||
| client.UserAgent = strings.Join(s, ";") |
There was a problem hiding this comment.
minor we'd tend to just format this directly:
client.UserAgent = fmt.Sprintf("%s;%s", client.UserAgent, azureAgent)
azurerm/config.go
Outdated
| client.UserAgent = fmt.Sprintf("HashiCorp-Terraform-v%s", version) | ||
|
|
||
| if azureAgent != "" { |
There was a problem hiding this comment.
minor we'd tend to make this:
if azureAgent := os.Getenv("AZURE_HTTP_USER_AGENT"); azureAgent != "" {
// set it
}
rather than separating it out :)
metacpp
requested review from
JunyiYi and
rcarun
and removed request for
rcarun
azurerm/config.go
Outdated
| @@ -190,6 +191,12 @@ func withRequestLogging() autorest.SendDecorator { | |||
| func setUserAgent(client *autorest.Client) { | |||
| version := terraform.VersionString() | |||
| client.UserAgent = fmt.Sprintf("HashiCorp-Terraform-v%s", version) | |||
|
|
|||
| if azureAgent := os.Getenv("AZURE_HTTP_USER_AGENT"); azureAgent != "" { | |||
There was a problem hiding this comment.
Do you want to add a comment here to explain it ?
There was a problem hiding this comment.
pushed a commit to add this.
|
@tombuildsstuff after it's merged, when it will be released to public ? We want to let the BI team start the monitoring after the release. |
|
Confirmed this matches the environment variable within CloudShell: |
|
@metacpp I'm not sure exactly when - but we'll do a release at some point early next week :) |
|
@tombuildsstuff thanks for the quick merge. @jeffreyCline will follow up on the release of plugin. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
No description provided.